-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Doing some updates to the magpie #6925
Conversation
db8fccb
to
49f9c7f
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
ctx := context.Background() | ||
err = ch.PublishWithContext( | ||
ctx, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is weird that ctx wasn't defined before.. how was it compiling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super wierd! Is this a global variable defined somewhere? If that's the case we should clean it up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should fix that. Doesn't have to be this PR.
Storing a context in a global variable like that is going to lead to a lot more confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it in this PR
49f9c7f
to
e6b787f
Compare
e6b787f
to
ad69f5c
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
ad69f5c
to
82af438
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
test/magpiego/go.mod
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrading all modules to keep things up-to-date
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for doing the cleanup.
c72b0ac
to
467d4f1
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Signed-off-by: ytimocin <[email protected]>
Signed-off-by: ytimocin <[email protected]>
467d4f1
to
00fd2d1
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Description
Opened this PR not from the fork because there are some actions that I would like to test and can't be triggered from a fork.
Type of change
Fixes: Upgrade modules in magpie and do a cleanup #6932
Auto-generated summary
🤖[deprecated] Generated by Copilot at db8fccb
Summary
🐰📦🚀
Updated the magpiego bindings for RabbitMQ and servicebus to use context and improve performance and readability. Also upgraded some dependencies in
go.mod
to use the latest versions.Walkthrough